Skip to content

drivers: usb: udc: dwc3: initial implementation#84544

Open
josuah wants to merge 1 commit into
zephyrproject-rtos:mainfrom
tinyvision-ai-inc:pr-dwc3
Open

drivers: usb: udc: dwc3: initial implementation#84544
josuah wants to merge 1 commit into
zephyrproject-rtos:mainfrom
tinyvision-ai-inc:pr-dwc3

Conversation

@josuah
Copy link
Copy Markdown
Contributor

@josuah josuah commented Jan 25, 2025

Add support for Synopsys DWC3 core.

There is no hardware implementing this core in this PR, and users are encouraged to include this commit it on downstream PRs, then I will periodically re-integrate the modifications they needed in this PR.

This can help synchronizing the work of everyone on a common basis.

Active branches using this:

Related issues/PRs:

@josuah josuah marked this pull request as draft January 25, 2025 04:46
@zephyrbot zephyrbot added the area: USB Universal Serial Bus label Jan 25, 2025
@josuah josuah added area: Drivers Experimental Experimental features not enabled by default labels Jan 25, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions Bot added the Stale label May 5, 2025
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented May 5, 2025

Once USB3 support lands (I anticipate after device -> device_next migration?) and once we could test it with IRQ support, I will reopen this PR.

If anybody need this before that happen, do feel free to ping me about it.

@github-actions github-actions Bot removed the Stale label May 6, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 6, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions Bot added the Stale label Jul 6, 2025
@github-actions github-actions Bot closed this Jul 20, 2025
@natto1784
Copy link
Copy Markdown
Contributor

can this (and the dependency) be reopened? i feel like this should be a welcome addition

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Jan 28, 2026

@natto1784 yes absolutely. However it might need a platform with which this can be tested with, otherwise it is dead code.

Do you know of any available hardware that can be picked for testing it?

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Jan 28, 2026

It seems like you are working on AM243x?

Here is the current state of this driver:

  • It is still being developed in rare occasions internally at tinyVision.ai for the needs of the https://tinyclunx33.tinyvision.ai/ (which remains a downstream platform with no current plan of upstreaming, not my choice and could change some day maybe)
  • Isochronous endpoint support might be on the roadmap
  • It lacks essential features but works well for as long as we used it with control and bulk endpoints
  • It requires the USB 3 support to be merged, but it is also possible to upstream an USB2-only version in the meantime.

I am @josuah.demangeon on Discord if that helps.

@natto1784
Copy link
Copy Markdown
Contributor

natto1784 commented Feb 12, 2026

Hi, @josuah, after some efforts with minimal changes/fixes to your driver (+ a small glue driver of our own), I have managed to get the CDC ACM sample working on AM62x SK platform which is already in the Zephyr tree, so perhaps we can get this PR re-opened, or is there some other approach to compiling patches in one patchset that you would prefer? I am open to talking on discord as well if that is what you prefer, my nickname is @ amneesh on the Zephyr discord.

In the meantime can you tell me the rationale behind adding the ev_* DT regions? They do not seem to be a standard thing and I have also removed them in my code.

Note that I got it working with the next stack and have not tested with the legacy one, but it should work regardless.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

Thank you for trying and making it so far!

It looks like there are two parallel efforts to get DWC3 working:

https://github.com/piterzhang/zephyr/tree/dwc3-rk3588

Luckily, both platforms use the same basis for the driver, so the implementation should be relatively similar.

can get this PR re-opened

If it was, I would need to be the one pushing the commits, so best if you open a new one.

have not tested with the legacy one, but it should work regardless.

The legacy stack uses different driver APIs, so it will not work with the legacy stack, but this is not a bad thing as it is expected to be deprecated relatively soon.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

DT_INST_REG_ADDR_BY_NAME() "REG" here stands for "Register", as in reg = <0x.....>;.

ev_* is what LiteX uses, an FPGA-based SoC architecture, so not relevant to DWC3 driver.

It is best to ignore it.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

I will ping you in the "DWC3 Upstreaming" thread...

@jfischer-no jfischer-no reopened this Feb 12, 2026
@jfischer-no jfischer-no removed the Stale label Feb 12, 2026
@jfischer-no
Copy link
Copy Markdown
Contributor

can get this PR re-opened

If it was, I would need to be the one pushing the commits, so best if you open a new one.

Let's keep it open until there is a new one. Alternatively, you can allow to push to this branch.

Comment thread drivers/usb/udc/udc_dwc3.c Outdated
@@ -0,0 +1,1593 @@
/*
* SPDX-License-Identifier: MIT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josuah Why is license MIT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be the fallback license that tinyVision.ai used for everything, I can request it to be changed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@natto1784
Copy link
Copy Markdown
Contributor

natto1784 commented Feb 12, 2026 via email

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

get rid of it as it's vendor specific code

Yes, totally, it better not be there and was planned to be removed for something more generic.

@josuah josuah force-pushed the pr-dwc3 branch 2 times, most recently from eb1629d to 3ee1194 Compare February 12, 2026 19:08
@jfischer-no
Copy link
Copy Markdown
Contributor

@josuah Do you want to have USB3 support in the stack, or will you limit this driver to support high-speed only and add USB3 support in the stack later?

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Apr 1, 2026

@jfischer-no I was thinking of keeping it USB2, and have a separate USB3 PR based on top of this DWC3 PR.

This way all the noise about DWC3 hardware can be kept away from USB3 protocol discussions.

If anything else is preferred let me know!

@natto1784
Copy link
Copy Markdown
Contributor

natto1784 commented Apr 6, 2026

I for one, do not mind HS-only DWC3 driver first before adding SS support. Also I think we can open this for review soon, would be great if we could get this merged for 4.5.

@natto1784
Copy link
Copy Markdown
Contributor

Hi, has there been some progress on using udc_setup_received yet? I can volunteer to make the change if you're bit tied up with stuff.

@josuah josuah force-pushed the pr-dwc3 branch 2 times, most recently from 81a1786 to 1dadb54 Compare May 9, 2026 00:48
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented May 9, 2026

Force-push: refactor according to #103493

Tested on a CrossLinkU-NX33 FPGA, but no result so far (needs vendor quirks for it).

Comment thread drivers/usb/udc/udc_dwc3.c Outdated
* ACK/NAK the first packets from the host with the new address, otherwise the host
* issue a reset
*/
if (cfg->setup_pkt->bmRequestType == 0x00 && cfg->setup_pkt->bRequest == 0x05) {
Copy link
Copy Markdown
Contributor

@natto1784 natto1784 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (cfg->setup_pkt->bmRequestType == 0x00 && cfg->setup_pkt->bRequest == 0x05) {
if (cfg->setup_pkt->bmRequestType == USB_REQTYPE_TYPE_STANDARD && cfg->setup_pkt->bRequest == USB_SREQ_SET_ADDRESS) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. This is still WIP but that is one thing less to lookup :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread drivers/usb/udc/udc_dwc3.c Outdated
udc_dwc3_do_set_address(dev, cfg->setup_pkt->wValue);
}

udc_setup_received(dev, cfg->setup_pkt);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: You can also directly use the programmed setup buffer in your EP0 queue by passing NULL here, I wonder which one is a better approach

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that the stack would submit the setup buffer, so yes this can be simplified by one less special case.

Maybe drivers that use a local buffer for setup are those which constantly need a buffer ready for setup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a lot better thank you.

Comment thread drivers/usb/udc/udc_dwc3.c Outdated
Comment on lines +1337 to +1339
/* Latency optimization: set the address immediately to be able to be able to
* ACK/NAK the first packets from the host with the new address, otherwise the host
* issue a reset
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set addr_before_status controller controller capability.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was missing thank you.

@josuah josuah added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label May 10, 2026
@josuah josuah force-pushed the pr-dwc3 branch 4 times, most recently from ec56b9f to 18f7fba Compare May 11, 2026 21:59
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented May 11, 2026

Force-push: fix all runtime bugs I could find when running CDC-ACM. Still some TODO left through the code.

  • migrate to the new UDC API
  • get CDC ACM working and fix the bugs
  • Remove all the Lattice-specific registers to make our own vendor quirks.
  • I import the changes suggested by @piterzhang, @natto1784 and test again on tinyCLUXN33
  • opening the PR for early review (with In Progress label still)
  • @piterzhang, @natto1784 can suggest a vendor quirk ready to merge
  • run usbtest

@josuah josuah force-pushed the pr-dwc3 branch 2 times, most recently from c178cfa to 4e6960f Compare May 12, 2026 01:12
Add support for Synopsys DWC3 core, with only BULK endpoints supported at
the moment, as well as an initial vendor quirks system.

Signed-off-by: Josuah Demangeon <me@josuah.net>
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented May 12, 2026

I finished migrating this to be using IRQs only without a workqueue thread.

The tinyCLUNX33 platform still does not have working IRQs, and uses a 1ms timer to simulate this (called from the tinyCLUNX33 vendor quirk, not part of this PR).

This means all the event handlers are only tested in non-IRQ context, and some modification might be needed to let all of this run from IRQ, such as reintroducing a work queue if needed.

Let me know what error you encounter.

@josuah josuah marked this pull request as ready for review May 12, 2026 01:23
@zephyrbot zephyrbot added area: Tests Issues related to a particular existing or missing test area: Devicetree Binding PR modifies or adds a Device Tree binding labels May 12, 2026
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Drivers area: Tests Issues related to a particular existing or missing test area: USB Universal Serial Bus Experimental Experimental features not enabled by default In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants